Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve Panelview Function #736

Merged
merged 9 commits into from
Dec 14, 2024
Merged

Improve Panelview Function #736

merged 9 commits into from
Dec 14, 2024

Conversation

rafimikail
Copy link
Contributor

@rafimikail rafimikail commented Dec 8, 2024 •
edited
Loading

What:

  • Fix panelview plot in documentation
  • Enable dynamic plot colors for panelview when collapse_by_cohort = True

@review-notebook-app Review Notebook App
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@codecov Codecov
Copy link

codecov bot commented Dec 8, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Flag Coverage Δ
core-tests 78.02% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
pyfixest/did/visualize.py 95.00% <100.00%> (+0.19%) ⬆️

@rafimikail
Copy link
Contributor Author

rafimikail commented Dec 8, 2024 •
edited
Loading

Tested on local
Before
image

After
image

@rafimikail
Copy link
Contributor Author

rafimikail commented Dec 8, 2024 •
edited
Loading

Hi @s3alfisc, this MR is to fix the bug on panelview plot size, as well as minor improvement to enable dynamic colors of pre and post treatment when setting collapse_to_cohort = True

I have ran the lint test locally and shows passed, not sure why the pre-commit workflow failed, is this expected? Need your help to review this, Thanks!

@s3alfisc
Copy link
Member

pre-commit.ci autofix

@s3alfisc
Copy link
Member

I have ran the lint test locally and shows passed, not sure why the pre-commit workflow failed, is this expected? Need your help to review this, Thanks!

Also not sure why this sometimes happens ... currently running the ci bot, I hope it will fix it =)

@s3alfisc
Copy link
Member

Looks good now, thanks a ton @rafimikail!

@s3alfisc s3alfisc merged commit 2cbfb63 into py-econometrics:master Dec 14, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@s3alfisc s3alfisc s3alfisc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rafimikail @s3alfisc